fix: pin type filtering in pin.ls#2228
Conversation
Old version returned indirect pin when `pin ls -t direct <path>` was executed. This fixes filtering and adds necessary tests. It also tweaks error handling to match behavior from go-ipfs/js-ipfs-http-client and pass interop tests from ipfs-inactive/interface-js-ipfs-core#375 License: MIT Signed-off-by: Marcin Rataj <[email protected]>
| waterfall([ | ||
| (cb) => resolvePath(self.object, paths, cb), | ||
| (hashes, cb) => mapSeries(hashes, (hash, done) => pin._isPinnedWithType(hash, types.all, done), cb), | ||
| (hashes, cb) => mapSeries(hashes, (hash, done) => pin._isPinnedWithType(hash, type, done), cb), |
There was a problem hiding this comment.
This is the fix for filtering 🙃
|
|
||
| if (!results.length) { | ||
| return cb(new Error(`Path is not pinned`)) | ||
| return cb(new Error(`path '${paths}' is not pinned`)) |
There was a problem hiding this comment.
Changed to match error from go-ipfs
Interop test for this is added in https://github.com/ipfs/interface-js-ipfs-core/pull/375/files#diff-6a00eb1307f801bb37999815584589bdR190
| result = await ipfs.pin.ls(path, { type }) | ||
| } catch (err) { | ||
| throw Boom.boomify(err, { message: 'Failed to list pins' }) | ||
| throw Boom.boomify(err) |
There was a problem hiding this comment.
Removed message prefix to match error from go-ipfs.
Interop test for this is added in https://github.com/ipfs/interface-js-ipfs-core/pull/375/files#diff-6a00eb1307f801bb37999815584589bdR199
| let repo | ||
|
|
||
| function expectPinned (hash, type, pinned = true) { | ||
| function expectPinned (hash, type = pinTypes.all, pinned = true) { |
There was a problem hiding this comment.
expectPinned was not checking if returned type match requested one, so I've added double-check for that.
|
|
||
| it('direct for path (no match)', function (done) { | ||
| pin.ls(`/ipfs/${pins.root}/mercury/wiki.md`, { type: 'direct' }, (err, pinset) => { | ||
| expect(err).to.exist() |
There was a problem hiding this comment.
To make it easier to maintain the error message is not hardcoded here, but checked in interop tests:
|
@alanshaw ok to merge this and ipfs-inactive/interface-js-ipfs-core#375 before the fix gets old again? :) |
This PR adds more detailed tests for `pin.ls`, including one that guards against issue described in ipfs-inactive/js-ipfs-http-client#875 refs ipfs/js-ipfs#2228 License: MIT Signed-off-by: Marcin Rataj <[email protected]>
License: MIT Signed-off-by: Alan Shaw <[email protected]>
License: MIT Signed-off-by: Alan Shaw <[email protected]>
3 participants
Summary
This PR fixes filtering, improves interop with go-ipfs and adds missing tests for
pin lsDetails
Old version returned indirect pin when
pin ls -t direct <path>was executed.This PR also tweaks error handling to match behavior from go-ipfs/js-ipfs-http-client and pass improved interop tests added in ipfs-inactive/interface-js-ipfs-core#375
I've added some inline comments, hope it helps in review.
Related
pin lsinterop tests: fix: pin.ls ignored opts when hash was present ipfs-inactive/interface-js-ipfs-core#375(this PR is not blocked by interop tests, old ones were less strict)